Allow "half" rating selection #2764

nicolaihenriksen · 2022-07-06T14:23:50Z

@Keboo Thanks for the feedback on this PR. I have reworked this PR based on your comments, and as such the changes are a bit more extensive; so grab a Mountain Dew before you read any further :-)

The PR now covers this:

Coercing Min/Max values. I don't see this as a breaking change since wrong values would yield an unusable rating bar.
Added opt-in functionality for previewing a value (before selecting it). Inspiration taken from Slider but works on hover in this case.
Added opt-in functionality for fractional values. This is done by adding a ValueIncrements property (double) which can have a value between double.Epsilon and 1.0. A value of 1.0 - which is the default - effectively means that fractional values are disabled. Specifying a value where less than 1.0 where 1 is not a multiple of said value (e.g. 0.75) works, but is not really useful. Perhaps the coercion of ValueIncrements should ensure that the 1 is a multiple of the value being set. Thoughts?
The two opt-in options are independent of each other; ie. one can be enabled without the other and vice versa.
Added a 3 purpose-built IMultiValueConveters, and placed them as nested classes in the RatingBar class because they are specifically made for that control. Let me know if you normally do it in a different manner.
Added unit tests for converters
Implementation respects RatingBar.Orientation and RatingBar.FlowDirection

Funny note:
The original RatingBar does not coerce its value, and thus you can set anything for the Value property. This is probably also needed, because with Min=1 and Max=5 you can effectively not (with the mouse at least) set a rating of "0 stars".
With fractional values enabled, I had to change this a bit, so in cases like that you would typically set Min=0 (instead of 1), which then means that you can partially select the first star; i.e. set a value between 0 and 1. In this case you can give a rating of "0 stars" because the coercion will eventually give you the value 0 if your mouse is close enough to the left edge of the first RatingBarButton.

Screen captures (GIFs) of the new opt-in features can be seen below:

Original PR text:

Partial fix for #2073

IMPORTANT:
Correct theming colors need to be applied in MaterialDesignTheme.RatingBar.xaml resources defined in the top of the file. @Keboo do you have good suggestions on how to get these colors (NOT brushes). And note that I also need some colors with a specific alpha-channel applied to them (matching 26% opacity). I hope something is already available to get this, otherwise a custom MarkupExtension could be an option.

Changes:
Changes RatingBar.Value to a double which is coerced into a multiple of 0.5 and forced to stay within the Min/Max bounds of the RatingBar. The "half selected" visual appearance is accomplished by using a (gradient) Brush on TextBlock.Foreground which is either fully opaque (selected), a mix of fully opaque and semi-transparent (half-way selected), or completely semi-transparent (not selected). These gradients/brushes are where I need to access theming colors.

Implementation respects RatingBar.Orientation, however I am not entirely sure (ie. I haven't tested) whether it works in RTL layout.

Value is now set as a double that is coerced to a multiple of 0.5 within the Min/Max bounds. The colors are currently hardcoded, this needs to be addressed. Unit tests should also be added for the coercion.

Fixed issue where the first rating button could not be halfway selected. It is worth noting that valid values are actually values between Min-1 and Max (in multiples of 0.5)

Keboo

This is great work, thank you for taking it on. A couple of suggestions.

First, I think we should make it an opt-in option for users clicking on the rating bar and getting fractional values. I think one way to accomplish this would be a property similar to the TickFrequency of the slider where the developer can indicate the level of precision that is desired.

As for getting the colors and setting it up. I think we might be able to support more than just 0.5 by leveraging some attached properties and a custom MultiValueConverter.

Here is a small sample I put together using a slider.

This XAML is using the Tag property, but we could create any number of internal attached/dependency properties to support this.

<Slider
    Minimum="0"
    Maximum="50"
    Value="35"
    Tag="{DynamicResource PrimaryHueMidBrush}">
    <Slider.Foreground>
        <MultiBinding Converter="{StaticResource LinearBackgroundConverter}">
            <Binding Path="Tag" RelativeSource="{RelativeSource Self}"/>
            <Binding Path="Value" RelativeSource="{RelativeSource Self}"/>
        </MultiBinding>
    </Slider.Foreground>
</Slider>

Then the converter would look something like this:

public object? Convert(object?[]? values, Type targetType, object parameter, CultureInfo culture)
{
    //TODO add orientation as another parameter (might also need min/max to derive percentage)
    if (values?.Length == 2 &&
        values[0] is SolidColorBrush brush &&
        values[1] is double value)
    {
        value /= 50;
        return new LinearGradientBrush()
        {
            StartPoint = new Point(0, 0.5),
            EndPoint = new Point(1, 0.5),
            GradientStops = new()
            {
                new GradientStop { Color = Colors.Fuchsia, Offset = value },
                new GradientStop { Color = brush.Color, Offset = value }
            }
        };
    }
    return null;
}

I think if we do something like this, it could also support more than just half values. Thoughts?

Keboo · 2022-07-08T05:01:54Z

MaterialDesignThemes.Wpf.Tests/RatingBarTests.cs

+public class RatingBarTests
+{
+    [StaTheory]
+    [InlineData(-5, 0.0)]


Should the expected value here be 1.0 to match the Min?

Well hidden in my commit comments, I left this little comment:

"It is worth noting that valid values are actually values between Min-1 and Max (in multiples of 0.5)"

I agree this is somewhat strange, and I will work on changing the behavior.

It should however be noted that the implementation before this PR did not have any coercion (which is strange for a "range bound" control IMO), and thus allowed you to set any value.

In any case, the first button represents the Min value today, and that is a little strange because that actually means you should not be able to set a rating less than 1 star when Min=1; why should it not be allowed to set 0 stars (although difficult to do with the mouse!)? Anyway, I will see if I can rework this into something a little more intuitive while preserving back-compat.

nicolaihenriksen · 2022-07-08T12:25:54Z

I think if we do something like this, it could also support more than just half values. Thoughts?

Great idea with the MultiValueConverter and the trick of injecting the current brush via the Tag (or an attached property). That should work nicely and will - as you say - allow for "any" step size (ie. TickFrequency).

My only concern about opening up for TickFrequency being very small increments, is how this is "supported" in the UI (from the users perspective). Currently there is no visual feedback on hover, so lets say we use 0.1 increments, it for one can be really difficult to actually hit those with the mouse, but I think we should definitely add a hover effect which then "previews" which value you're about to apply (perhaps both with coloring in the RatingBarButtons up to the desired value, but maybe also a numeric indication of the value itself? Thoughts?

Keboo · 2022-07-08T17:04:28Z

My only concern about opening up for TickFrequency being very small increments, is how this is "supported" in the UI (from the users perspective). Currently there is no visual feedback on hover, so lets say we use 0.1 increments, it for one can be really difficult to actually hit those with the mouse, but I think we should definitely add a hover effect which then "previews" which value you're about to apply (perhaps both with coloring in the RatingBarButtons up to the desired value, but maybe also a numeric indication of the value itself? Thoughts?

I agree with the "supported" problem. I suspect that most consumers that use small increments will likely also set them to readonly (and I suspect bind the value to a TextBlock or similar to show a numeric value).

I do like the idea of allowing the user to see the bound value on hover or press. I think some interaction that is similar to how the slider does it might be nice. Though I would say that displaying the value should be an opt-in behavior.

Refactored into opt-in for fractional values. Fractional increments are now also configurable via the ValueIncrements DP.

Positioning of tooltip still missing

nicolaihenriksen · 2022-07-13T20:27:58Z

My only concern about opening up for TickFrequency being very small increments, is how this is "supported" in the UI (from the users perspective). Currently there is no visual feedback on hover, so lets say we use 0.1 increments, it for one can be really difficult to actually hit those with the mouse, but I think we should definitely add a hover effect which then "previews" which value you're about to apply (perhaps both with coloring in the RatingBarButtons up to the desired value, but maybe also a numeric indication of the value itself? Thoughts?

I agree with the "supported" problem. I suspect that most consumers that use small increments will likely also set them to readonly (and I suspect bind the value to a TextBlock or similar to show a numeric value).

I do like the idea of allowing the user to see the bound value on hover or press. I think some interaction that is similar to how the slider does it might be nice. Though I would say that displaying the value should be an opt-in behavior.

I have reworked the PR a bit to include opt-in features for fractional values (freely set), and preview of value being selected. See the updated PR description for details.

Keboo

This is really awesome work! Thank you

nicolaihenriksen added 5 commits July 6, 2022 15:54

RatingBar.Value changed to double

e597147

Value is now set as a double that is coerced to a multiple of 0.5 within the Min/Max bounds. The colors are currently hardcoded, this needs to be addressed. Unit tests should also be added for the coercion.

Added unit test for coercion

798b6bb

Merge branch 'MaterialDesignInXAML:master' into fix2073

671301a

Changed [Theory] to [StaTheory] to make unit tests run

c31f855

Fixed lower bound issue

4fa2dd6

Fixed issue where the first rating button could not be halfway selected. It is worth noting that valid values are actually values between Min-1 and Max (in multiples of 0.5)

Keboo reviewed Jul 8, 2022

View reviewed changes

nicolaihenriksen added 8 commits July 8, 2022 21:06

Merge branch 'MaterialDesignInXAML:master' into fix2073

848d95e

POC for MultiValueConverter approach

f860aec

Opt-in for fractional values

32b876e

Refactored into opt-in for fractional values. Fractional increments are now also configurable via the ValueIncrements DP.

opt-in for PreviewValue added (no visualization)

41ecce1

Moved multiconverter into nested class

d08e437

Added preview "tooltip"

4599ade

Positioning of tooltip still missing

Correct placement of value preview "tooltip"

9f9e0e6

Added unit tests for transform converters

43735d2

Marking RatingBarButton.IsWithinSelectedValue as obsolete

fea1dd9

Keboo approved these changes Jul 15, 2022

View reviewed changes

Keboo added this to the 4.6.0 milestone Jul 15, 2022

Keboo added enhancement release notes Items are likely to be highlighted in the release notes. labels Jul 15, 2022

Keboo merged commit 50cb0e8 into MaterialDesignInXAML:master Jul 15, 2022

nicolaihenriksen deleted the fix2073 branch July 15, 2022 14:30

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Allow "half" rating selection #2764

Allow "half" rating selection #2764

nicolaihenriksen commented Jul 6, 2022 •

edited

Loading

Keboo left a comment

Keboo Jul 8, 2022

nicolaihenriksen Jul 8, 2022

nicolaihenriksen commented Jul 8, 2022

Keboo commented Jul 8, 2022

nicolaihenriksen commented Jul 13, 2022

Keboo left a comment

Allow "half" rating selection #2764

Allow "half" rating selection #2764

Conversation

nicolaihenriksen commented Jul 6, 2022 • edited Loading

Original PR text:

Keboo left a comment

Choose a reason for hiding this comment

Keboo Jul 8, 2022

Choose a reason for hiding this comment

nicolaihenriksen Jul 8, 2022

Choose a reason for hiding this comment

nicolaihenriksen commented Jul 8, 2022

Keboo commented Jul 8, 2022

nicolaihenriksen commented Jul 13, 2022

Keboo left a comment

Choose a reason for hiding this comment

nicolaihenriksen commented Jul 6, 2022 •

edited

Loading